Skip to content

P0-8 sh:// path fix + P0-1/P0-3/P0-4/P0-5 follow-ups, README reduction (P1-5) - #98

Merged
yannrichet-asnr merged 20 commits into
mainfrom
claude/new-session-4b2704
Sep 30, 2026
Merged

yannrichet-asnr merged 20 commits into
mainfrom
claude/new-session-4b2704

Conversation

@yannrichet

@yannrichet yannrichet commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Description

Audit-plan work stacked on one branch (several tickets in one PR, by the maintainer's choice):

  1. P0-8 – sh:// path resolution. The resolver converted every word that looked like a file name into an absolute path in the launch directory without checking existence. With sh://cat in.txt > out.txt the calculation read the un-substituted template from the launch directory and wrote out.txt outside the case directory (shared across parallel cases), with no error: silently wrong results.
  2. P0-1 follow-ups – cache identity (code_id / version_cmd).
  3. P0-3 follow-up – remote interrupt command quoting.
  4. P0-4 follow-up – "no timeout" warning once per campaign.
  5. P0-5 – Python 3.14 promoted to CI's stable matrix and declared.
  6. P1-5 (partial) – CITATION.cff; README reduced from ~3 450 to ~290 lines; all documentation merged and deduplicated into doc/ (one file per topic).
  7. Skill (skills/fz/) and doc updates for the above.

Related Issues

Audit plan tickets P0-8, P0-1 (compléments), P0-3 (gap found during cross-check), P0-4 (affinage), P0-5, P1-5 (partial). No GitHub issue is referenced.

Type of Change

  • Bug fix – changes results of existing commands (see Breaking changes)
  • Security hardening
  • Test addition/modification
  • CI / metadata / documentation restructuring

Changes Made

P0-8 (fz/runners/sh.py): a word is resolved to the launch directory only if it exists there and not in the case directory (compiled inputs win); targets of >, >>, 2> … are never resolved (stricter than the ticket); each resolved word is logged at info level; sh://bash script.sh with the script only in the launch directory still works. tests/test_comprehensive_paths.py: the Windows-only expectation failed for the tar case was a symptom of the old resolver; now done everywhere.

P0-1 follow-ups

  • cache:// warns once per campaign when it ignores a legacy v1 cache entry (fz/io.py).
  • version_cmd with a non-zero exit no longer becomes the code_id (sh:// and ssh://); stderr with exit 0 still accepted.
  • Pre-existing bug fixed: version_cmd was executed with the value of FZ_SHELL_PATH (a list of directories) as the shell program, so it always failed when that variable was set (e.g. Windows CI). It now uses fz.shell.run_command.
  • "Threat Model" lists version_cmd as executed code; examples/Telemac alias declares a version_cmd.

P0-3 follow-up: the best-effort remote kill on interrupt built pkill -P $(pgrep -f '<pattern>') with the slurm:// partition (from the URI) and the ssh:// command prefix inside single quotes, unquoted: a ' allowed remote shell injection on that path. Now fz.runners.ssh.build_kill_cmd (shlex.quote). The version_cmd warning no longer prints a URI with an embedded password.

P0-4 follow-up: the "No timeout set for ssh:// / slurm:// calculations (unlimited)" warning is logged once per scheme and fzr() campaign (reset_timeout_warnings()) instead of per case.

P0-5: 3.14 added to the stable CI matrix on Linux, macOS and Windows (the Ubuntu-only 3.14-dev job is removed) and the classifier declared; tests/test_python_version_support.py requires the two to match.

P1-5 (partial)

  • CITATION.cff (author confirmed by the maintainer; no version, DOI or ORCID).
  • README is now an entry point (features, installation, quick start, the six functions, key concepts, configuration essentials, Threat Model kept in full, AI-agent/MCP pointers, links). Old README.md#<section> links land on a "Former README sections" list (hidden anchors) that points to the new pages.
  • All documentation is in doc/, one file per topic, deduplicated. Each former README section was first moved unchanged, then compared with the existing page of the same topic: content already covered was dropped, only what the page lacked was merged (e.g. factorial vs non-factorial designs and output type casting in core-functions.md; progress callbacks and SSH keepalive in parallel-and-caching.md; calculator-model compatibility in calculators.md; plugin creation in installing-models.md; full output structure in overview.md). Topics without an existing page became new files (cli-usage.md, configuration.md, custom-algorithms.md, installation.md, quick-start.md, interrupt-handling.md, breaking-changes.md, troubleshooting.md, development.md, ai-agents.md, resources.md); doc/INDEX.md maps topics to files. Also deduplicated: interrupt handling (single page), shell path and environment variables (configuration.md ↔ shell-path.md, calculators.md). Editorial judgement was involved (what counts as "already covered"), so this part deserves a human read.
  • Documentation errors found and fixed while merging (checked against the code or by running it): fzr(callbacks=...) takes a dict of named callbacks (on_start, on_case_start, on_case_complete, on_progress, on_complete), not a list of functions (a list raises TypeError); there is no fz list algorithms|models CLI command nor fz.list_algorithms(); an alias without an entry for the requested model does not raise a "does not support model" error (the bare URI is used, so an sh:// case fails with "Permission denied"); the .fz_hash example now shows the v2 format.
  • Guards: tests/test_readme_structure.py (README <= 300 lines, links), tests/test_docs_consistency.py (every FZ_* variable cited in the docs exists in the code, relative links in doc/ resolve, former-README content present, no leftover "Guide:" sections, callbacks documented as a dict, legacy TOC anchors kept).
  • Not done: MkDocs site, DOI, conda-forge, positioning page, JOSS paper; cli-usage.md still partly overlaps the per-function CLI snippets of core-functions.md; a few pre-existing doc code blocks are not valid Python as written (elided arguments, Jupyter magics).

Skill / docs: skills/fz/ documents the sh:// resolution rule, version_cmd semantics/trust, the legacy-cache warning and the executed-code threat model; NEWS.md, CLAUDE.md, llms.txt updated (and a broken llms.txt link fixed).

Testing Performed

  • New tests: test_p0_8_sh_path_resolution.py (5), test_cache_code_id.py (+5), test_version_cmd_ssh_exit_status.py (3, mocked paramiko), test_p0_3_kill_cmd_quoting.py (9), test_run_timeout.py (+1), test_project_metadata.py (1), test_readme_structure.py (2), test_docs_consistency.py (6). Regression tests were checked to fail without their fix where applicable.
  • Kill-path tests: the real _execute_remote_command and _execute_remote_slurm_command are driven with a mocked SSH client and a simulated interrupt; the captured kill command is executed in a real bash with fake pgrep/pkill. With the old interpolation both tests create the injected PWNED file; with build_kill_cmd they pass. Skipped on Windows.
  • Documentation guards pass locally (Linux); the callbacks API, the .fz_hash format and the alias/model behaviour documented in this PR were verified by running fz.
  • Cross-check of the audit's "Fait" rows by reading code and running tests locally: P0-3, P0-4, P0-7 (20 passed with mcp installed).
  • CI was fully green on da4dcaf (Linux/macOS/Windows 3.9–3.14, MSYS2, CLI, examples, SSH localhost, SLURM, Funz Calculator, lint, docs, wheels). Commits since then (kill tests, timeout warning, README reduction, docs consolidation and deduplication, 6205d14) await the next CI run. The headless Claude Code e2e job is skipped, so tests/test_skill_e2e.py is not exercised.
  • Not covered: real SSH/SLURM servers for version_cmd and the kill command (mocked client only); GitHub's handling of the hidden legacy anchors. Local full-suite run (earlier): only environment-related failures (yq flavour in two test_python_outputs tests; one permission test because the sandbox runs as root).

Breaking Changes

  • Potentially different results, by correction: sh:// commands that referenced input/output files by bare name now use the compiled file and write in the case directory. Results obtained earlier with such commands should be re-checked.
  • Documentation links: README sections moved into doc/ (old README.md#<section> links land on the "Former README sections" list). No API change.

Additional Notes / known, not fixed here

  • fz appends the input file name after the command (cat in.txt > res.txt in.txt). Pre-existing; changing it affects every sh:// command.
  • The "password in URI" warning is deduplicated per (host, message) per process, not per campaign as the audit text says.
  • The remote kill is best-effort and coarse (pgrep -f on a command prefix / srun.*<partition> can match unrelated processes); only its quoting was fixed here.
  • Other audit rows (P0-2 wording, P0-6 fz.api, P1-x) not re-verified; a corrected audit status document was produced separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y

…r and absent from case dir

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…f the old resolver)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…fix failing version_cmd

- cache:// warns once per campaign when it ignores a legacy (v1) cache entry
- README Threat Model lists version_cmd as executed code
- version_cmd exiting non-zero no longer yields its error message as code_id
- Telemac example alias declares a version_cmd; tests cover it

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…che warning, threat model

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
@yannrichet yannrichet changed the title Fix: path resolution in sh:// commands reads wrong files (P0-8) Fix sh:// path resolution (P0-8) + P0-1 cache identity follow-ups Sep 30, 2026
…S bash)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…ata tests

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…ramiko client

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…t URI in version_cmd warning

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
@yannrichet yannrichet changed the title Fix sh:// path resolution (P0-8) + P0-1 cache identity follow-ups P0-8 sh:// path resolution fix + P0-1/P0-3/P0-5 follow-ups Sep 30, 2026
@yannrichet
yannrichet marked this pull request as draft September 30, 2026 16:50
@yannrichet
yannrichet marked this pull request as ready for review September 30, 2026 16:52
…able CI matrix versions may be declared); drop redundant test

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
@yannrichet yannrichet changed the title P0-8 sh:// path resolution fix + P0-1/P0-3/P0-5 follow-ups P0-8 sh:// path resolution fix + P0-1/P0-3 follow-ups, CITATION.cff Sep 30, 2026
…are its classifier

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…and run the captured kill command in bash

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
@yannrichet yannrichet changed the title P0-8 sh:// path resolution fix + P0-1/P0-3 follow-ups, CITATION.cff P0-8 sh:// path resolution fix + P0-1/P0-3/P0-5 follow-ups, CITATION.cff Sep 30, 2026
…ad of once per case

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…tent unchanged to doc/guide/

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
@yannrichet yannrichet changed the title P0-8 sh:// path resolution fix + P0-1/P0-3/P0-5 follow-ups, CITATION.cff P0-8 sh:// path fix + P0-1/P0-3/P0-4/P0-5 follow-ups, README reduction (P1-5) Sep 30, 2026
…, document sh:// file resolution, add consistency tests

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…doc/guide/

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…o reference pages), fix documented-but-wrong API statements

- fzr(callbacks=...) is a dict of named callbacks, not a list
- no 'fz list algorithms/models' CLI nor fz.list_algorithms()
- alias without an entry for the model: bare URI is used (no 'does not support model' error)
- .fz_hash example shows the v2 format; output structure moved into overview.md

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…nk core-functions to cli-usage; add python-block validity test

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
@yannrichet-asnr
yannrichet-asnr merged commit 70432ce into main Sep 30, 2026
39 checks passed
yannrichet pushed a commit that referenced this pull request Sep 30, 2026
…uctured doc/

Re-applies the unmerged review of claude/fz-docs-skills-review-8xkpgw on
top of #98 (doc/ restructuring) and P0-8 (sh:// path resolution), after
re-checking every finding against current main by running fz:

- doc/limitations.md (new): verified constraints and pitfalls; sh:// part
  updated for P0-8 (argument appending remains a trap)
- fzr examples passing calculators as 4th positional argument (results_dir)
- default delimiters (() for variables without delim), no ?var conversion
- fzc per-case sub-directories, fzo on case dirs; skill ladder fixed
- FZ_RUN_TIMEOUT=0, first Ctrl+C terminates running cases, cache://_
- os.environ after import -> reload_config(); fz.shell imports
- funz:// UDP port, SSH auth/host keys, fz list and --global caveats
- notebook 02: ?(name) needs varprefix '?'

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GxLbauyVHBQPCeSow8hdh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants